Skip to content

Unify Isaac Lab assets with Isaac Sim #3608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Unify Isaac Lab assets with Isaac Sim #3608

wants to merge 9 commits into from
+88 −92

Conversation

stevfeng
Copy link

@stevfeng stevfeng commented Oct 2, 2025

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (existing functionality will not work without user modification)
  • Documentation update

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@github-actions github-actions bot added asset New asset feature or request isaac-sim Related to Isaac Sim team labels Oct 2, 2025
@stevfeng stevfeng requested a review from matthewtrepte October 2, 2025 18:32
@Mayankm96
Copy link
Contributor

How was the asset verification done? Did you run training on a proper set of environments and made sure they all train exactly the same?

Mayankm96
Mayankm96 requested changes Oct 7, 2025
View reviewed changes
Copy link
Contributor

@Mayankm96 Mayankm96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concerns with how asset generation was done and verifed. This is a big change.

@stevfeng
Copy link
Author

stevfeng commented Oct 7, 2025

Concerns with how asset generation was done and verifed. This is a big change.

How was the asset verification done? Did you run training on a proper set of environments and made sure they all train exactly the same?

Matthew helped me verify all these assets using a benchmark with 2 main metrics: rewards and duration. We verified that all the tasks have similar metrics as before

@matthewtrepte to provide more details as well

@stevfeng stevfeng requested a review from Mayankm96 October 7, 2025 20:29
kellyguo11
kellyguo11 reviewed Oct 13, 2025
View reviewed changes
apps/isaaclab.python.headless.kit
persistent.isaac.asset_root.default = "https://omniverse-content-production.s3-us-west-2.amazonaws.com/Assets/Isaac/5.0"
persistent.isaac.asset_root.cloud = "https://omniverse-content-production.s3-us-west-2.amazonaws.com/Assets/Isaac/5.0"
persistent.isaac.asset_root.nvidia = "https://omniverse-content-production.s3-us-west-2.amazonaws.com/Assets/Isaac/5.0"
persistent.isaac.asset_root.default = "https://omniverse-content-staging.s3-us-west-2.amazonaws.com/Assets/Isaac/5.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't be updating this on main, probably better to point this PR to release/2.3.0 branch instead

source/isaaclab/isaaclab/utils/assets.py
import omni.client

NUCLEUS_ASSET_ROOT_DIR = carb.settings.get_settings().get("/persistent/isaac/asset_root/cloud")
NUCLEUS_ASSET_ROOT_DIR = (carb.settings.get_settings().get("/persistent/isaac/asset_root/cloud")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this change needed?

@kellyguo11
Copy link
Contributor

@matthewtrepte do you have any training curves to show the before/after for the impacted environments?
Have we also ran through all the demo and check scripts that use these assets?

@stevfeng please also run the formatter to address the linter errors.

@Mayankm96 Mayankm96 moved this to In review in Isaac Lab Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kellyguo11 kellyguo11 kellyguo11 left review comments

@ooctipus ooctipus Awaiting requested review from ooctipus ooctipus is a code owner

@pascal-roth pascal-roth Awaiting requested review from pascal-roth pascal-roth is a code owner

@jtigue-bdai jtigue-bdai Awaiting requested review from jtigue-bdai jtigue-bdai is a code owner

@hhansen-bdai hhansen-bdai Awaiting requested review from hhansen-bdai hhansen-bdai is a code owner

@matthewtrepte matthewtrepte Awaiting requested review from matthewtrepte

@Mayankm96 Mayankm96 Awaiting requested review from Mayankm96 Mayankm96 is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

asset New asset feature or request isaac-sim Related to Isaac Sim team

Projects

Isaac Lab
Status: In review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@stevfeng @Mayankm96 @kellyguo11